feat(analytics): use the seed-derived auth pubkey as the Amplitude/Sentry user_id#939
Merged
Conversation
getAuthUserId() returns the auth keypair's public key hex for analytics identification, or null when the session is locked. Only the public hex ever leaves getAuthKeypair — private key material is never exposed.
…ntry) getUserId() now prefers the seed-derived auth id (services/auth/getAuthUserId) when the session is unlocked, persisting/overwriting the stored METRICS_USER_ID so existing users migrate off the random id. Falls back to the existing stored/random id when locked. identifyUser()'s dedup and Amplitude setUserId, plus App.tsx's Sentry.setUser, both already resolve through getUserId() so they follow the auth id automatically. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Uses the seed-derived auth public key as the stable analytics and Sentry identity.
Changes:
- Adds public auth-ID derivation.
- Persists the auth ID with random-ID fallback.
- Adds derivation and migration tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/services/auth/getAuthUserId.ts |
Derives the public auth ID. |
src/services/analytics/user.ts |
Prefers and persists the auth ID. |
__tests__/services/auth/getAuthUserId.test.ts |
Tests auth-ID resolution. |
__tests__/services/analytics/user.test.ts |
Tests persistence and fallback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
iOS Simulator preview build is ready: https://github.com/stellar/freighter-mobile/releases/tag/untagged-29720b731b669f3d9d3a (SDF collaborators only — install instructions in the release description) |
Addresses two Copilot review findings on PR #939. 1. Migration never fired on the real cold-start path. The sole production identifyUser() call is the mount effect in useAnalyticsPermissions, whose host (RootNavigator) stays mounted across lock->unlock, so getUserId() resolved the random-id fallback on a locked cold start and nothing re-identified after unlock. Add an effect that reconciles identity when auth transitions into AUTHENTICATED; identifyUser() dedups so it is a no-op once migrated. 2. Static import of services/auth/getAuthUserId closed the cycle analytics/user -> auth/getAuthUserId -> auth/getAuthKeypair -> ducks/auth -> services/analytics(index) -> analytics/user, which can snapshot a partially-initialized `analytics` export. Defer it to a call-time require() in getUserId(), matching the pattern in auth/attachAuth.ts. Tests: new useAnalyticsPermissions lifecycle test (3/3); user.ts suite still 4/4; lint:ts clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The analytics user id is the seed-derived auth pubkey (== the backend JWT `sub`): a stable, cross-service, reinstall-surviving identifier. Sentry is initialized unconditionally and beforeSend keeps events (only trimming context) when analytics are disabled, and nothing cleared the Sentry user on opt-out — so after a user disabled data sharing, the persisted auth id kept riding along on crash telemetry. (Pre-PR the leaked value was an unlinkable random id; adopting the auth id elevated its sensitivity.) - updateSentryContext now owns the Sentry user identity, setting it only when analytics is enabled and clearing it (setUser(null)) otherwise. It already re-runs on the analytics toggle and auth transitions via useSentryContext, so opt-out clears immediately and the persist-hydration race a one-shot launch read would hit is avoided. - App.tsx's cold-start Sentry.setUser is guarded on the same consent flag. Mirrors the extension, which only calls Sentry.setUser when data-sharing is allowed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CassioMG
reviewed
Jul 22, 2026
CassioMG
approved these changes
Jul 22, 2026
CassioMG
left a comment
CassioMG
left a comment
Contributor
There was a problem hiding this comment.
Lgtm - left a non-blocking suggestion of a small refactor
…concileAnalyticsUserId Addresses CassioMG's non-blocking review suggestion on #939: keep getUserId storage-only and resolve the seed-derived auth id in a dedicated module outside the analytics barrel — removing the call-time require() cycle break and aligning mobile with the extension's architecture. - getUserId reverts to its pre-PR storage-only form (no auth import, no require()). - New services/analytics/reconcileUserId.ts (downstream of the barrel, so plain static imports): resolves getAuthUserId, persists it, calls analytics.identifyUser(), then updateSentryContext(). The trailing updateSentryContext() closes the Sentry one-session lag — useSentryContext doesn't re-run on the store userId changing, so previously the auth id only reached Sentry next launch; now it lands in-session (consent-gated by the updateSentryContext change). - useAnalyticsPermissions' AUTHENTICATED effect calls reconcileAnalyticsUserId instead of bare identifyUser. - Tests: retire user.test.ts (its auth-preference assertions move to the new reconcileUserId.test.ts); useAnalyticsPermissions.test.tsx asserts reconcile is invoked on the unlock transition. jest + lint:ts clean (confirms no init-order cycle regression). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Sets the wallet's seed-derived auth public key as the Amplitude and Sentry
user_id, replacing the random per-install id — and migrates existing users to it automatically. The mobile counterpart to the extension change (stellar/freighter): the same stable, cross-platform id the backend uses (sub), byte-identical to the extension's.Only the public key hex is read/persisted/emitted — the private keypair never leaves
getAuthKeypair. No new data-collection surface.Implementation details (for agents/reviewers)
What changed:
src/services/auth/getAuthUserId.ts—getAuthUserId(): Promise<string | null>: the public auth key hex viagetAuthKeypair()(keypair.rawPublicKey().toString("hex")),nullwhen locked. The keypair never escapes.src/services/analytics/user.ts—getUserId()now resolves the auth id first (persisting/overwriting the stored id) with the existing random/session path preserved verbatim as the locked-state fallback. The existingidentifyUser()(dedup +amplitude.setUserId) reconciles/migrates automatically: an existing user's stored random id → auth id,setUserIdfires once.src/components/App.tsx— no change needed; its SentrysetUseralready sourcesidfromgetUserId(), so it adopts the auth id (self-heals to the persisted id on the launch after first reconcile).Verification: TDD, per-task spec+quality reviews + a whole-branch final review (verdict: ready to merge, no Critical/Important).
getAuthUserId2/2 + auth suite 55;user.ts4/4 + full suite 2681;lint:tsclean. A test asserts the migration overwrite and that the random fallback path is unchanged when locked; the "never leaks the keypair" property is verified.Follow-ups (deferred, non-blocking):
App.tsxsets Sentry's id once at mount, so a mid-session unlock lags Sentry to the next launch (Amplitude reconciles viaidentifyUser); pre-existing.AsyncStoragewrite when the stored id already equals the auth id.Depends on the shipped
deriveAuthKeypair/getAuthKeypair(src/services/auth). Extension counterpart: stellar/freighter.